-
Notifications
You must be signed in to change notification settings - Fork 3.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Tensor: Standardize Vector2, Vector3, Vector4, Color3, Color4, Quaternion, and Matrix #14235
Conversation
Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s). |
Snapshot stored with reference name: Test environment: To test a playground add it to the URL, for example: https://babylonsnapshots.z22.web.core.windows.net/refs/pull/14235/merge/index.html#WGZLGJ#4600 Links to test babylon tools with this snapshot: https://playground.babylonjs.com/?snapshot=refs/pull/14235/merge To test the snapshot in the playground with a playground ID add it after the snapshot query string: https://playground.babylonjs.com/?snapshot=refs/pull/14235/merge#BCU1XR#0 |
Added FlattenTuple
Fixed Flatten Added comment for Vector Formatting
Small note - there are a few type issues, building core was not successful. |
@kzhsw Thank you so much for the reviews. Another set of eyes helps to catch things missed. |
Updated MultidimensionalArray to correctly handle nesting
I love it! |
When this plan will be realized, this design is useful |
Fixed some type paramters
Visualization tests for WebGPU (Experimental) |
The only way I know how to manage static members with interfaces is this: I am not the biggest fan of this way, but this is the only way (I know :-)) to define static members for a class. @sebavan - we can discuss this offline and see whether or not this solves the issue. |
I've separated the instance and static sides into interfaces for both I also looked into defining I apologize for not responding sooner @RaananW, I was experimenting with the
The way I prefer, which also preserves the class syntax, is with the export class Vector2 implements Vector<Tuple<number, 2>> {
// ...
}
Vector2 satisfies VectorStatic<Vector2>; Please note that the left-hand side of |
Added Vector2 and Vector3 RandomToRef Removed DistanceOfPointFromSegment from Vector Added static side checks to Vector2 and Vector3
Visualization tests for WebGPU (Experimental) |
Moved Clamp and ClampToRef from Vector to Tensor
Let s use @RaananW proposal to do it instead of satisfies as it has only been introduced really late in TS and could create compat issues with the users I guess. |
Typescript < 4.9 is already not usable with BJS so using
|
I am not a big fan of any of these ways. Each way of validating statics has its downside. the downside of satisfies is that it fails not as part of the class. It's an assertion line (i know you can claim any typing is basically assertion). So the error is shown during the assertion, and not during the class declaration. The typescript version that Seb mentioned is not necessarily the version we use to compile the framework, it is the language that typescript developers can use. If not using WebGPU you would be able to continue using the same typescript version that you were using, if that's your decision. It is also possible to add the definition of the missing declaration. However, satisfies is a typescript keyword that doesn't allow it. We try our best not to do that. |
Unfortunately using interface StaticSide {
DoSomething(): void;
}
const Class: StaticSide = class {
DoSomething() {
// ...
}
OtherThing() {
// ...
}
}
Class.OtherThing(); // <-- Error! OtherThing does not exist on StaticSide This can be solved by using a separate variable though:
|
have you seen this - Of course, if you implement it the way you showed it wouldn't work :-) Again - there is no good solution here. I am, in general, not in favor of any of those solutions. I have the least resistance to this solution. |
Since |
I double checked and satisfies does not appear in .d.ts so at least static thingies do not pollute emitted runtime code. The only issue is the lack of detailed error message for us in dev: |
I'll be honest - this discussion is way too deep for something we won't change for a long time :-) Whatever fits. Satisfies as assertion feels a bit off, but typescript does support it. Other things are as hacky as it is. I am fine with satisfies (in THIS case) for the same reason as I wrote above - Vectors won't change any time soon. @dr-vortex - I am getting to the last question on my side, and it is perf. Let's run a few more perf tests, sharing what we use to run them, to know 100% we don't hurt anything (and maybe even improve a bit). This won't be affected by any interface/satisfies changes. |
I'm for it! Let's merge it |
If you have no other concerns could you please approve the PR (it currently says you are requesting changes)? Thank you very much! |
Waiting on @RaananW perf check so it prevents accidental merge |
@sebavan - want to resolve your review? there are still open change requests |
@RaananW approved but let us know your perf results |
Every scene I have tested was on par with the current implementation. I tested heavy scenes (like the performance priority PG and the physics stress testing), and the PG i pasted before testing the function implementations directly (including creation and basic math). |
Discussion of this PR is available on the Babylon.js forum in this thread.
Resolves #14201
TL;DR:
Tensor
would be added, which provides a standardized API for interacting with vectors, colors, matrices, quaternions, etc.Rational
This PR aims to create a standard interface from which various math constructs are defined.
Take the current difference in behavior between vectors and colors:
And the lack of a method that should exist in Color3:
Tensor
provides a single interface for tensor-like objects which users can depend on. By having tensor-like classes implementTensor
, it ensures that all the methods exist and have the same and correct behavior. It is meant for organization and standardization.Semantics
Note:
CurrentClass
is a placeholder for a class that implementsTensor
. In fields, it is the current class.Tensor
and its related types will be defined insrc/Maths/tensor.ts
unless otherwise notedSignature<T extends number>
may be shown when it is implemented asSignature<T> = T extends number ? ... : never
Status
Vector
Constructor
MultidimensionalArray
TensorValue
Tensor
Vector
Tensor.dimension
Tensor.From
Tensor.as
Tensor.sum
Tensor.rank
Tensor.value
isTensor
isTensorValue
getTensorDimension
convertTensor
** May not be included in the proposal
Vector
Defined in
src/Maths/math.vector.ts
This PR includes
Vector
from #13699.Vector
extendsTensor
and adds normalization methods,length
, andlengthSquared
. It is implemented byVector2
,Vector3
, andVector4
.Tuple manipulation and math types
Defined in
src/types.ts
The following types are being added for compile-time math and tuple manipulation, which is necessary for
MultidimensionalArray
. They are included as a single block for brevity.Constructor
Defined in
src/types.ts
Defines a single type for constructors. This replaces
Vector2Constructor
,Vector3Constructor
,Vector4Constructor
,QuaternionConstructor
, andMatrixConstructor
.Instead of using
Vector3Constructor<this>
, you would instead useConstructor<typeof Vector3, this>
. Thetypeof
is needed to get the type for the class instead of the instance, and can't be done inside the type since Typescript prohibits taking thetypeof
a type.MultidimensionalArray
Defined in
src/types.ts
Represents a multidimensional array of
T
with depthD
.TensorValue
Tensor
Tensor
is adeclare class
for tensor-like classes to implement.Tensor
includes all of the methods inVector
from #13699. This includesadd
,subtract
,multiply
,divide
,scale
, ...)fromArray
,toArray
,asArray
, ...)number[]
, the return type for array-related methods isnumber[]
. SeeTensor.value
clone
,copyFrom
,copyFromFloats
, ...)InPlace
,ToRef
,FromFloats
, etc.It adds or changes the following methods:
Tensor.dimension
dimension
is the mathematical dimension2 of the tensor. In dynamic tensor types, it can be defined as a getter.Example:
Tensor.From
From
creates a new instance ofCurrentClass
fromsource
.source
: The tensor to copy data from.fillValue
: The value to use for filling empty parts of the resultingCurrentClass
.Example:
Additionally,
From
could be changed toFrom(...args: [...Tensor[], number]): CurrentClass
to allow multiple inputs. This is better understood by this invalid Typescript signature (since rest parameters must be last):Tensor.as
as
creates an instance oftype
from an instance ofCurrentClass
.type
: The class to create an instance of.fillValue
: The value to use for filling empty parts of the resulting instance.Example:
Tensor.sum
sum
return the sum of the components of theTensor
.Example:
Tensor.rank
The
rank
of a tensor is the number of indices required to uniquely select each element of the tensor.Example:
Tensor.value
value
is the values of the tensor in a multidimensional array with the typeV
(the type parameter of the class). Unlikedimension
, this must be implemented as a getter and setter.Example:
isTensor
Checks if a value is a
Tensor
.Example:
isTensorValue
Checks if a value is a
TensorValue
.Example:
getTensorDimension
getTensorDimension
returns the mathimatical dimension2 ofvalue
, similar toTensor.dimension
. Ifvalue
is not aTensor
orTensorValue
, it will throw aTypeError
(optional) convertTensor
Creates a new
T
with the values oftensor
. UnlikeTensor.as
, it performs more error checking and does not need a starting instance (and therefore avoids the dreaded'methodName' does not exist on undefined
error).Behavior changes
Color3
andColor4
methodsmultiplyToRef
,scaleToRef
,scaleAndAddToRef
,clampToRef
,addToRef
, andsubtractToRef
have had their return types changed fromthis
to the reference instance.Considerations
Add
,Normalize
,Lerp
) and non-static normalization methods are outside the scope of this PR. WhileTensor
may include them in the future, this PR does notTensor
.Tensor
is defined using thedeclare class
and classes that followTensor
do so using theimplements
keyword, there are no runtime changes toTensor
-based classes.isTensor
,getTensorDimension
, etc.). The TS declaration size will also increase slightly due toTensor
and the added utility types.Questions
For the BabylonJS team concerning the PR
Size
) would be included and standardized?FAQs
What use cases does this proposal have?
Tensor
is not intended to be used by users. It is meant for internal organization and standardization. The functions (isTensor
,getTensorDimension
, etc.) are intended to be public and may be used by users. Their use cases may be inferred from their descriptions.What is the difference between this and #13699?
The capabilities of the type.
Vector
supports any rank 1 tensor.Tensor
generalizesVector
, and supports any rank tensor. A mathematical vector can be represented asnumber[]
. A mathematical tensor can be represented as anumber
,number[]
,number[][]
, and so on. This table highlights the capability:number[]
Vector
[number, number]
Vector2
[number, number, number]
Vector3
,Color3
[number, number, number, number]
Vector4
,Color4
,Quaternion
number[][]
Matrix
number[][][]
number[][][][]
TensorValue
How is incompatibility managed? Are incompatible classes dropped or are they modified to follow
Tensor
?Any classes included will be modified to follow the standard. In many cases, the diverging behavior makes the engine difficult to use, and changing the claseses to follow the
Tensor
standard benefits end users.Further reading
1. https://en.wikipedia.org/wiki/Tensor
2. https://en.wikipedia.org/wiki/Dimension